Skip to content

docs(helm): document supported values and add usage examples#280

Open
chihkang wants to merge 3 commits intoopenabdev:mainfrom
chihkang:docs/helm-values-reference
Open

docs(helm): document supported values and add usage examples#280
chihkang wants to merge 3 commits intoopenabdev:mainfrom
chihkang:docs/helm-values-reference

Conversation

@chihkang
Copy link
Copy Markdown

@chihkang chihkang commented Apr 13, 2026

Summary

This PR improves the Helm chart documentation by adding several supported but currently undocumented values to the values reference and README.

Users can already rely on these options in practice, but today they are difficult to discover unless they inspect the chart templates directly. This change makes the chart easier to use and reduces trial-and-error during deployment.

Closes #163.

Changes

  • add fullnameOverride to values.yaml
  • add nameOverride to values.yaml
  • document envFrom in values.yaml comments and chart docs
  • document agentsMd usage with --set-file
  • document the discord.allowedChannels --set-string requirement more prominently
  • add a chart-level Helm values reference with practical examples
  • link the main README Helm install section to the chart values reference

Why

The chart already supports these configuration paths, but they are not clearly documented. That creates unnecessary friction for users, especially for:

  • multi-instance deployments
  • secret-based environment injection
  • large AGENTS.md configurations
  • Discord channel ID configuration

Notes

This PR is documentation-focused and does not change chart behavior. It only makes existing supported options easier to discover and use.

Testing

  • reviewed chart templates to confirm the documented values are already supported
  • documentation change only; helm CLI was not available in the local environment for helm lint

@chihkang chihkang requested a review from thepagent as a code owner April 13, 2026 07:29
@thepagent thepagent added the p3 Low — nice to have label Apr 15, 2026
chaodu-agent
chaodu-agent reviewed May 1, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@chaodu-agent chaodu-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Needs Update — Good intent, but stale + merge conflicts + incomplete coverage.

Baseline Check (Step 0)
Field Value
State OPEN
Mergeable CONFLICTING
Created 2026-04-13 (18 days ago)
Changes +78 / -0, docs only
Labels p3, closing-soon

Main branch status: No charts/openab/README.md exists — PR creates it fresh. ✅ However, values.yaml has changed significantly since April 13 with ~19 feature commits touching the chart.

Four-Question Framework

1. What problem does it solve?
The Helm chart supports many values only discoverable by reading templates directly. This PR adds a chart-level README with values reference table and practical usage examples.

2. How does it solve it?

  • Creates charts/openab/README.md with values table + 4 usage examples
  • Adds top-level nameOverride and fullnameOverride defaults to values.yaml
  • Adds inline comments for envFrom and agentsMd

3. What was considered?
Documentation-only, no behavior change. Contributor reviewed templates to confirm documented values are supported.

4. Is it the best approach?
Sound approach — chart-level README is standard Helm convention. But execution has gaps due to staleness.

Traffic Light

🟢 INFO

  • Correct identification of undocumented gap (nameOverride/fullnameOverride missing from values.yaml)
  • Practical examples (envFrom, --set-file agentsMd, Discord ID precision warning)
  • Standard Helm convention for chart docs

🟡 NIT

  1. Values table very incomplete — Documents only ~8 values. Actual chart has 27+ agent-level keys including slack.*, gateway.*, reactions.*, stt.*, cron.*, persistence.*, pool.*, extraInitContainers, extraContainers, allowBotMessages, trustedBotIds, allowDm, maxBotTurns, etc.
  2. Consider helm-docs for auto-generation from values.yaml comments.

🔴 SUGGESTED CHANGES

  1. Merge conflict must be resolved — values.yaml has changed significantly since April 13.
  2. Stale documentation risk — Since PR opened, main gained: gateway templates, maxBotTurns, allowDm, reactions.toolDisplay, cron/cronjobs, extraContainers, Slack adapter config, allowBotMessages/trustedBotIds. The "values reference" title is misleading at ~15% coverage.
  3. closing-soon label — Missing Discord Discussion URL. Will auto-close in 3 days unless addressed.

Reviewed by 超渡法師 🔃 chaodu Backlog triage

@chaodu-agent
Copy link
Copy Markdown
Collaborator

chaodu-agent commented May 2, 2026

超渡法師 Review — PR #280

1. What problem does it solve?

The Helm chart supports several useful values (fullnameOverride, nameOverride, envFrom, agentsMd, --set-string for Discord IDs) but none were documented in a discoverable way. Users had to read _helpers.tpl and template source to find them.

2. How does it solve it?

  • New charts/openab/README.md — 66-line values reference with tables and three practical examples (name override, envFrom for secrets, --set-file for AGENTS.md, Discord ID precision warning).
  • charts/openab/values.yaml — adds nameOverride: "" and fullnameOverride: "" as top-level defaults (consumed by _helpers.tpl but never declared).
  • README.md — cross-reference linking to the chart README from Quick Start.

3. What was considered?

Docs-only, no behavior changes. Contributor reviewed chart templates to confirm all documented values are already functional.

4. Is this the best approach?

Clean, well-scoped docs PR that directly addresses every item in issue #163. Surfacing existing values in values.yaml defaults is standard Helm convention.

Traffic Light

🟢 INFO — Well done

  • nameOverride/fullnameOverride added to values.yaml — verified that _helpers.tpl on main already consumes both. helm show values will now surface them.
  • The --set-string precision warning for Discord IDs is a real footgun; documenting it prominently is valuable.
  • envFrom example correctly shows both secretRef and configMapRef patterns.
  • Cross-reference from root README to chart README is a nice touch.

🟡 NIT

  1. Values table is incomplete — The table omits commonly-used agent values already in values.yaml: pool, reactions, persistence, stt, gateway, slack, allowBotMessages, trustedBotIds. The table doesn't need to be exhaustive, but a note like "For the full list, run helm show values openab/openab" would set expectations.
  2. Chart-level vs agent-level nameOverride — The existing commented-out agent blocks already have a per-agent nameOverride. A one-line comment like # Chart-level name override (for per-agent override, see agents.<name>.nameOverride) would prevent confusion.
  3. allowedUsers default — Shown as [] with description "Empty means allow all users." Consider making this clearer: "Empty = allow all (default)".
Verdict

🟢 Looks good — Clean docs-only PR. The NITs above are minor improvements. No blocking issues.

@shaun-agent
Copy link
Copy Markdown
Contributor

shaun-agent commented May 3, 2026

OpenAB PR Screening

This is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Click 👍 if you find this useful. Human review will be done within 24 hours. We appreciate your support and contribution 🙏

Screening report ## Intent

PR #280 is trying to make the OpenAB Helm chart easier to deploy by documenting supported configuration values that already work but are currently hard to discover.

The operator-visible problem is deployment friction: users need to inspect chart templates or guess Helm syntax for common cases like name overrides, secret-based env injection, large AGENTS.md configs, and Discord channel IDs.

Feat

This is a documentation improvement with a small chart values-reference cleanup.

Behavioral change: no runtime behavior is intended to change. The PR adds or documents Helm values and usage examples:

  • fullnameOverride
  • nameOverride
  • envFrom
  • agentsMd with --set-file
  • discord.allowedChannels with --set-string
  • chart-level Helm values reference
  • README link to the chart documentation

Who It Serves

Primary beneficiaries: deployers and maintainers operating OpenAB through Helm.

Secondary beneficiaries: reviewers and support maintainers, because clearer deployment docs reduce repeated clarification around supported chart values and Helm CLI edge cases.

Rewritten Prompt

Review and merge a documentation-focused Helm chart update for OpenAB.

Confirm that every documented value is already supported by the existing chart templates, especially fullnameOverride, nameOverride, envFrom, agentsMd, and discord.allowedChannels. Ensure examples use correct Helm syntax, including --set-file for large AGENTS.md content and --set-string for Discord channel IDs. Verify the new chart README is linked from the main README and that no chart behavior changes are introduced beyond documenting existing supported values.

If Helm is available, run helm lint charts/openab; otherwise, perform template-level review and note that Helm CLI validation remains a follow-up.

Merge Pitch

This PR is worth advancing because it improves deployability without changing runtime behavior. The risk profile is low: the affected files are documentation and values comments, with the only practical risk being inaccurate documentation if a listed value is not actually supported by the chart.

Likely reviewer concern: whether the new examples are guaranteed to match current chart behavior and whether adding nameOverride / fullnameOverride to values.yaml could be interpreted as a behavior change rather than surfacing existing Helm conventions.

Best-Practice Comparison

OpenClaw principles relevant here:

  • Explicit delivery routing is indirectly relevant: documenting discord.allowedChannels helps operators route bot behavior to the intended Discord channels.
  • Durable job persistence, isolated executions, retry/backoff, run logs, and gateway-owned scheduling are not directly relevant because this PR is Helm documentation, not runtime scheduling or execution behavior.

Hermes Agent principles relevant here:

  • Self-contained prompts for scheduled tasks are loosely analogous to documenting agentsMd clearly, especially the --set-file pattern for larger configuration.
  • Atomic writes, file locking, daemon tick models, and fresh sessions per scheduled run are not relevant to this PR.

The best-practice takeaway is that operational systems should make routing and configuration explicit. This PR fits that principle by turning implicit chart support into visible deployment guidance.

Implementation Options

Option 1: Conservative documentation-only merge
Accept the PR if template review confirms the documented values already exist. Optionally ask the author to add a note that no behavior changes are intended.

Option 2: Balanced docs plus validation
Merge the documentation, but also run or request helm lint charts/openab and possibly helm template examples for the documented paths. This gives reviewers stronger confidence without expanding scope much.

Option 3: Ambitious Helm documentation hardening
Use this PR as the start of a fuller Helm chart docs pass: generated values reference, tested example commands, CI linting for chart changes, and sample values files for common deployment modes such as Discord-only, Slack-only, and multi-instance installs.

Comparison Table

Option Speed to ship Complexity Reliability Maintainability User impact Fit for OpenAB right now
Conservative documentation-only merge High Low Medium Medium Medium Good
Balanced docs plus validation Medium Low-Medium High High Medium-High Best
Ambitious Helm docs hardening Low Medium-High High High High Good later, too large for this PR

Recommendation

Advance with Option 2: merge the documentation after validating that the documented values are already supported by the chart and that the examples render correctly.

This keeps the PR appropriately scoped while addressing the main reviewer risk: documentation accuracy. If Helm is unavailable in the review environment, Masami or Pahud should either run helm lint charts/openab locally or leave a clear follow-up asking for chart linting in CI. Broader Helm docs automation should be split into a separate follow-up rather than added to this PR.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

@chihkang
Copy link
Copy Markdown
Author

chihkang commented May 4, 2026

Updated this PR and pushed a refreshed branch (8c51547).

What changed:

  • Merged current main into docs/helm-values-reference and resolved the charts/openab/values.yaml conflict.
  • Kept the docs-focused scope while refreshing charts/openab/README.md against the current chart values.
  • Renamed the chart docs section from Values Reference to Common Values and added a pointer to helm show values openab/openab / values.yaml for the full option list.
  • Clarified chart-level vs per-agent nameOverride and the default meaning of discord.allowedUsers.
  • Covered the newly added value families called out in review, including Slack, gateway, reactions, STT, cron, persistence, and extra container/volume hooks.

Local validation with Helm v3.18.5:

  • helm lint charts/openab passed
  • helm template openab charts/openab passed
  • Smoke render with fullnameOverride, envFrom, --set-file agentsMd, and --set-string Discord channel ID passed
  • helm unittest charts/openab passed: 7 suites, 74 tests

GitHub now shows the PR Discussion URL check passing. I do not have permission to remove the remaining closing-soon label, so that may need maintainer/bot cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chaodu-agent chaodu-agent chaodu-agent left review comments

@thepagent thepagent Awaiting requested review from thepagent thepagent is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 3 days p3 Low — nice to have

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

docs: add Helm values reference with undocumented options

4 participants

@chihkang @chaodu-agent @shaun-agent @thepagent